Switch to dev.cel:cel
#303
Conversation
| buf("build.buf:buf:${libs.versions.buf.get()}:${osdetector.classifier}@exe") | ||
|
|
||
| testImplementation(libs.assertj) | ||
| testImplementation(libs.grpc.protobuf) |
There was a problem hiding this comment.
For google.rpc.Status
There was a problem hiding this comment.
In some of the cel protos we generate code for
| standard_rules/bytes: | ||
| - pattern/invalid/not_utf8 | ||
| # input: [ type.googleapis.com/buf.validate.conformance.cases.BytesPattern ]:{val:"\x99"} | ||
| # want: runtime error: value must be valid UTF-8 to apply regexp | ||
| # got: validation error (1 violation) | ||
| # 1. rule_id: "bytes.pattern" | ||
| # message: "value must match regex pattern `^[\\x00-\\x7F]+$`" | ||
| # field: "val" elements:{field_number:1 field_name:"val" field_type:TYPE_BYTES} | ||
| # rule: "bytes.pattern" elements:{field_number:15 field_name:"bytes" field_type:TYPE_MESSAGE} elements:{field_number:4 field_name:"pattern" field_type:TYPE_STRING} |
There was a problem hiding this comment.
Can be solved if we override the bytes_to_string: #304
| Program program = ruleEnv.program(rule.astExpression.ast, globals, PARTIAL_EVAL_OPTIONS); | ||
| Program.EvalResult evalResult = program.eval(Activation.emptyActivation()); | ||
| Val value = evalResult.getVal(); | ||
| if (value != null) { | ||
| Object val = value.value(); | ||
| if (val instanceof Boolean && value.booleanValue()) { | ||
| continue; | ||
| } | ||
| if (val instanceof String && val.equals("")) { | ||
| continue; | ||
| } | ||
| } | ||
| Ast residual = ruleEnv.residualAst(rule.astExpression.ast, evalResult.getEvalDetails()); | ||
| programs.add( | ||
| new CompiledProgram( | ||
| ruleEnv.program(residual, globals), | ||
| rule.astExpression.source, | ||
| rule.rulePath, | ||
| ruleValue)); |
There was a problem hiding this comment.
From an internal feature standpoint this is the only step we skipped.
I am also not sure if it ever worked. It seems we have to tell which variables/attrs are not available by passing a PartialActivation type to eval. See here.
timostamm
left a comment
timostamm
left a comment
There was a problem hiding this comment.
Looks great so far, but I wasn't able to review everything in this pass.
| /** | ||
| * CEL supports protobuf natively but when we pass it field values (like scalars, repeated, and | ||
| * maps) it has no way to treat them like a proto message field. This class has methods to convert | ||
| * to a cel values. | ||
| */ | ||
| final class ProtoAdapter { |
| * for accessing the variable `now` that's constant within an evaluation. | ||
| */ | ||
| class NowVariable implements Activation { | ||
| class NowVariable implements CelVariableResolver { |
There was a problem hiding this comment.
IIRC, we had a bug early on where now was cached for a validator instance, instead of for a validation pass. The conformance tests can't catch it. Would be good to be sure we don't regress here.
There was a problem hiding this comment.
I did not know that. I checked the references for NowVariable class, it is only created here alongside this variable.
conformance/src/test/java/build/buf/protovalidate/ValidatorTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Sri Krishna <[email protected]>
Signed-off-by: Sri Krishna <[email protected]>
Signed-off-by: Sri Krishna <[email protected]>
| buf("build.buf:buf:${libs.versions.buf.get()}:${osdetector.classifier}@exe") | ||
|
|
||
| testImplementation(libs.assertj) | ||
| testImplementation(libs.grpc.protobuf) |
Co-authored-by: Steve Ayers <[email protected]>
Switch to Google's CEL implementation.
-operator and using Floats doesn't work with any function.eval.Deliberately didn't refactor to keep relatively easier to review.